USE 557 - Parallelized input record read and bitstream download#10
Merged
Conversation
Why these changes are being introduced: A primary flow in this app is reading input records from TIMDEX, currently filtered to theses records, downloading fulltext for them, and then writing back to the TIMDEX dataset. The writing back is not yet established, so for now we write to JSONLines as debugging output. But the rest of the business logic is ready to be built, building on the CLI scaffolding. Parallelization is important. If a single item takes 1 second, for ~150k items in DSpace that's ~41 hours. Not realistic. With parallelization of ~20 workers, and retries and backoffs to ensure we don't get rate limited at the DSpace boundary, seeing more like ~33 records per second, which works out to ~1 hour. It's likely that an agressive run could bump `--workers=50` (or even higher) and ramp this up. How this addresses that need: Introduces new `dfh/harvest.py` file encapsulates parallelized reading of TIMDEX records, generation of pre-signed URLs from DSpace, and actual bitstream downloads from S3. This parallelization has sensible defaults, but is configureable via some limited CLI arguments (e.g. `--workers`). The parallelization lumps the generation of URLs and downloading of content into a single method with retries. This allows the parallelization to be relatively simple, and leave retries to the method. Ultimately, an iterator of record metadata + fulltext is yielded, which is what we'll need to write back to TIMDEX dataset. NOTE: DSpace credential parsing is still assuming env vars are set for host, username, and password. It's likely that we'll parse an env var with JSON credentials; this is not yet implemented. Side effects of this change: * None Relevant ticket(s): * https://mitlibraries.atlassian.net/browse/USE-557
ghukill
changed the title
Why these changes are being introduced: Intermittent failures were observed for authentication against a cold API endpoint, where after a successful authentication, all future requests were healthy. Could be an artifact of server deploys and upgrades (this codebase is landing during a DSpace migration), or an artifact of DSpace CRIS, unsure. Either way, this authentication check + retry feels pretty harmless to layer on until we're 100% sure it's not needed. How this addresses that need: * Adds new `warm_dspace_auth()` function * Applied at CLI level for `harvest` command before real API work Side effects of this change: * A cold DSpace API, when given some retries to authenticate, is successful after that. Relevant ticket(s): * https://mitlibraries.atlassian.net/browse/USE-557
ghukill
force-pushed
the
USE-557-cli-harvest-orchestration
branch
from
3419d5a to
dcb11b6
Compare
ehanson8
approved these changes
May 18, 2026
ehanson8
left a comment
ehanson8
left a comment
There was a problem hiding this comment.
Aside from fixing our favorite typo, looks great!
Comment on lines
+123
to
+124
| "fulltext_bistream_uuid": bitstream_uuid, | ||
| "fulltext_bistream_content": fulltext, |
Collaborator
Author
There was a problem hiding this comment.
ACK. Now, not just a comment or docstring....
Good catch, thank you! I'm going to add a keystroke logger on my machine that shocks me anytime I accidentally type "bistream"
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose and background context
Why these changes are being introduced:
A primary flow in this app is reading input records from TIMDEX (currently hardcoded to filter to theses records) downloading fulltext for them, and then writing back to the TIMDEX dataset. The writing back is not yet established, so for now we write to JSONLines as debugging output. But the rest of the business logic is ready to be built, building on the CLI scaffolding.
Parallelization is important. If a single item takes 1 second, for ~150k items in DSpace that's ~41 hours. Not realistic. With parallelization of ~20 workers, and retries and backoffs to ensure we don't get rate limited at the DSpace boundary, seeing more like ~33 records per second, which works out to ~1 hour. It's likely that an agressive run could bump
--workers=50(or even higher) and ramp this up. The primary bottleneck is network I/O, making this a good fit for threads.How this addresses that need:
Introduces new
dfh/harvest.pyfile that encapsulates parallelized reading of TIMDEX records, generation of pre-signed URLs from DSpace, and actual bitstream downloads from S3. This parallelization has sensible defaults, but is configurable via some limited CLI arguments (e.g.--workers).The parallelization lumps the generation of URLs and downloading of content into a single method with retries. This allows the parallelization to be relatively simple, and leave retries to the method.
Ultimately, an iterator of record metadata + fulltext is yielded, which is what we'll need to write back to TIMDEX dataset.
NOTE: DSpace credential parsing is still assuming env vars are set for host, username, and password. It's likely that we'll parse an env var with JSON credentials; this is not yet implemented.
How can a reviewer manually see the effects of these changes?
1- Set
TimdexMangersDev1 credentials2- Set env vars (DSpace credentials shared offline of PR):
3- Run CLI:
Includes new or updated dependencies?
YES
Changes expectations for external applications?
NO
What are the relevant tickets?
Code review